Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs(proposal): externaldns api graduation to beta #5079

Conversation

ivankatliarchuk
Copy link
Contributor

@ivankatliarchuk ivankatliarchuk commented Feb 9, 2025

Description

Relates, but not resolves #2941

The proposal to merge to master 2025-Mar-09 with the decision

Checklist

  • Unit tests updated
  • End user documentation updated

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 9, 2025
@ivankatliarchuk
Copy link
Contributor Author

/label tide/merge-method-squash

@k8s-ci-robot k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Feb 9, 2025
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 17, 2025
Copy link
Contributor

@Raffo Raffo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the proposal to graduate DNSEndpoint. I would encourage to try to understand the status of the API today, its limits and the required changes before drawing any conclusion on what needs to be changed. I think we need to do this before the beta graduation as ideally the beta to GA transition should be mostly a noop. I am not too up to speed on how this is used, maybe we can open an issue to survey users or trying a github search and see if people are using it in public repositories.

@ivankatliarchuk
Copy link
Contributor Author

ivankatliarchuk commented Feb 21, 2025

I like the proposal to graduate DNSEndpoint. I would encourage to try to understand the status of the API today, its limits and the required changes before drawing any conclusion on what needs to be changed. I think we need to do this before the beta graduation as ideally the beta to GA transition should be mostly a noop. I am not too up to speed on how this is used, maybe we can open an issue to survey users or trying a github search and see if people are using it in public repositories.

That make sense. Shell I rename the proposal?

### Goals
- Define the necessary requirements for `DNSEndpoint` API to reach beta status.
- Improve API stability, usability, and documentation.
- Improve test coverage, automate documentation creation, and validation mechanisms.
- Ensure backward compatibility and migration strategies from alpha to beta.
- Collect and incorporate feedback from existing users to refine the API.
- Address any identified issues or limitations in the current API design.

### Goals

- Define the necessary requirements for `DNSEndpoint` API to reach beta status.
- Improve API stability, usability, and documentation.
- Improve test coverage, automate documentation creation, and validation mechanisms.
- Ensure backward compatibility and migration strategies from alpha to beta.
- Collect and incorporate feedback from existing users to refine the API.
- Address any identified issues or limitations in the current API design.

### Non-Goals

- This proposal does not cover the graduation of ExternalDNS itself to a stable release.
- Making `DNSEndpoint` a stable (GA) API at this stage.
- It does not include implementation details for specific DNS providers.
- It does not introduce new functionality beyond stabilizing the DNSEndpoint API.
- Redesigning the API from scratch.
- Introducing breaking changes that would require significant refactoring for existing users.

Goals are mainly review, and implement pre-requisits before we decide on actuall migration. Expectation is, when all the goals that we define completed, I have not planned the future to be honest.

@Raffo
Copy link
Contributor

Raffo commented Feb 25, 2025

@ivankatliarchuk that's good, maybe we can add a bit more Information on how do we expect to gather feedback so that we can make this more actionable?

1 similar comment
@Raffo
Copy link
Contributor

Raffo commented Feb 25, 2025

@ivankatliarchuk that's good, maybe we can add a bit more Information on how do we expect to gather feedback so that we can make this more actionable?

@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 26, 2025
* master: (80 commits)
  chore(deps): bump the dev-dependencies group across 1 directory with 7 updates
  Update README.md with proper link to dev guide
  Add OpenStack Designate webook provider to readme
  chore(deps): bump the dev-dependencies group with 3 updates
  chore(deps): bump the dev-dependencies group with 20 updates
  chore(deps): bump azure/setup-helm in the dev-dependencies group
  style: formatting
  fix: remove broken test
  fix test name
  chore: upgrade ExternalDNS to go 1.24
  chore-makefile-coverage
  cover source.go getProviderSpecificAnnotations() with unit tests
  nitpick: rename cloudflare custom hostname test function
  review suggestions based improvements
  review suggestions
  fix(source): debug log on gateway target detection
  improve error message phrasing
  Update docs/sources/service.md
  chore(formatting): fix infected files with correct formatting (kubernetes-sigs#5099)
  docs: Fix managed-record-type argument
  ...
@ivankatliarchuk
Copy link
Contributor Author

@ivankatliarchuk that's good, maybe we can add a bit more Information on how do we expect to gather feedback so that we can make this more actionable?

Added to proposal

1. Capture feedback from the community on missing functionality for DNSEndpoint CRD
   - In a form of Github issue, pin the issue to the project
   - Link all CRD related issues to it

@ivankatliarchuk
Copy link
Contributor Author

/open

@ivankatliarchuk
Copy link
Contributor Author

/reopen

@k8s-ci-robot k8s-ci-robot reopened this Feb 26, 2025
@k8s-ci-robot
Copy link
Contributor

@ivankatliarchuk: Reopened this PR.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@ivankatliarchuk
Copy link
Contributor Author

Hi @Raffo and @szuecs. I've updated as per-comments. Anything is missing?

@Raffo
Copy link
Contributor

Raffo commented Mar 7, 2025

LGTM, @szuecs WDYT?

@szuecs
Copy link
Contributor

szuecs commented Mar 28, 2025

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 28, 2025
@ivankatliarchuk
Copy link
Contributor Author

cc @mloiseleur

- It does not include implementation details for specific DNS providers.
- It does not introduce new functionality beyond stabilizing the DNSEndpoint API.
- Redesigning the API from scratch.
- Introducing breaking changes that would require significant refactoring for existing users.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's definitely a non goal :D

@mloiseleur
Copy link
Collaborator

LGTM, only found some typos.
We can merge it once they are fixed.

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 28, 2025
@mloiseleur
Copy link
Collaborator

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 31, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mloiseleur

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 31, 2025
@k8s-ci-robot k8s-ci-robot merged commit f67cc94 into kubernetes-sigs:master Mar 31, 2025
13 checks passed
ivankatliarchuk added a commit to gofogo/k8s-sigs-external-dns-fork that referenced this pull request Apr 1, 2025
* master:
  docs(proposal): externaldns api graduation to beta (kubernetes-sigs#5079)
  chore(code-cleanup): move logic away from main.go add tests (kubernetes-sigs#5222)
  chore(deps): bump the dev-dependencies group across 1 directory with 17 updates
  chore: add se for nlb, alb in thailand region
  fix(node): logger test fixed (kubernetes-sigs#5232)
  fix(chart): add missing types for empty values (kubernetes-sigs#5207)
  docs: Fix typo: grcp → grpc.
  removing reduntant code
  renaming variable
  added new tests to handle edge case
  detailed documentation with no-expose
  added warn log
  edited docs and made new test
  docs: added documentation in node source
  fix: fixing ci lint
  fix: removing fmt.Printf
  feat: added expose internal ipv6 flag
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make DNSEndpoint CRD GA
5 participants